-
Notifications
You must be signed in to change notification settings - Fork 406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The compile_data
attribute can now be gathered from dependencies
#814
Conversation
Not sure if this is an expensive solution but I think it's fine? Hopefully reviewers will have some insight 😅 |
Do we really need to propagate Adding |
It's also used in binaries but maybe I misread this as a suggestion that it's only used by libraries.
The issue is that at the time it's the responsibility of the crate that's given How would I populate |
Additionally, I do think it might be valuable to be able to transitively provide data without having it end up in the runfiles for runtime. |
Yup it is used in binaries too for the same reason it is in libraries - to make a dependency available during the rustc invocation, not to be propagated.
Oh right that is true, we have to put compile data into the provider in case there is a wrapper rule. But I would still only put the compile data of the current target, not transitive ones.
I'd be interested to learn about an actual, real world use case for this. And even then, I think it would make sense to create a different attribute for transitive propagation. So target author can decide which behavior matches their use case. Propagating by default can end up being too memory costly in large workspaces. |
I don't think I have a reasonable real-world use case. I've removed the use of transitive compile-data. I think this covers the practical use case now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you! Only a kind unit test nerd snipe attempt remaining :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much! Looks great.
The
compile_data
attribute seems like it should propagate all the way through to the final build outputs since it'd be required by wrapper rules (rust_test
) to be able to successfully compile.closes #567